fix(preprocess): parse shared exchange ticker lists#129
Conversation
The files on this branch were committed with CRLF line terminators, which marked every line as changed and produced spurious merge conflicts with master. Convert them back to LF so the diff reflects only the real behavior change from LLMQuant#108. Co-Authored-By: Claude Opus 4.8 (1M context) <[email protected]>
keli-wen
left a comment
There was a problem hiding this comment.
Requesting changes. Thanks for taking on #108 — the intent is right, but the shared-prefix extension as written is a net regression, and I don't think this change should carry a new example or a design-doc rewrite. Details below.
Blocking: the shared-prefix scan over-captures ordinary prose
The continuation loop keys off match.end() and then searches for the next ) anywhere in the text (scan_text.find(")", match.end())). When the main regex already matched a balanced (EXCHANGE: SYMBOL) — it consumed its own closing paren — the loop still runs and scans past that paren into unrelated following text. Because _SHARED_EXCHANGE_SYMBOL_RE is compiled with re.IGNORECASE, an ordinary lowercase word right after a (EXCHANGE: SYMBOL), is captured as a ticker.
This flows straight into preprocess_news_document (ticker_hints, news.py:330), so it pollutes real output. On current master the input below yields one hint; on this branch it yields a bogus second one:
# (symbol, exchange, raw) projected from each returned NewsTickerHint
>>> extract_exchange_ticker_hints("Shares of (NYSE: IBM), and (NASDAQ: AAPL) rose today.")
('IBM', 'NYSE', '(NYSE: IBM)')
('AND', 'NYSE', 'NYSE: AND') # <- "and" captured as ticker "AND"
('AAPL', 'NASDAQ', '(NASDAQ: AAPL)')
The trigger — a balanced (EXCH: SYM), followed by any later ) — is extremely common in PR-wire prose, so this regresses far more documents than the ~1 warrant of recall that #108 estimates it recovers. The added tests only assert the five happy-path shapes from the issue and never exercise this case, so CI stays green while the regression ships.
Related smell: raw is meant to be the literal matched substring (provenance), but continuation members set raw=f"{exchange}: {symbol}" — a string that never appears in the source (NYSE: EVEXW is reconstructed) — while the first member keeps a dangling (NYSE: EVEX. The tests encode both, which locks that broken raw contract in.
This change should not add an example
extract_exchange_ticker_hints already exists and is already demonstrated end-to-end by examples/preprocess/01_news_pr_wire.py. Extending an existing operation's parsing coverage is a fix, not a new public operation, so it should ship regression tests rather than a second example file. Please drop examples/preprocess/02_news_ticker_lists.py. We'll also tighten the contributor guidance so the "ships with an example" expectation is scoped to new public operations, not edge-case extensions.
The contexts/ doc change is questionable here too
Whether a parsing edge-case should rewrite contexts/design/flow/news.md is debatable — unless this actually changes a documented contract, the design doc probably shouldn't move. It is also the only real merge conflict with master right now (#130 rewrote that file), so dropping the doc change both narrows scope and clears the conflict.
Suggested path
Narrow this to what #108 is actually worth: keep the existing simple/balanced capture and add regression tests that pin the boundary — the first symbol of a shared-prefix list is captured, and (EXCH: SYM), ...) does not over-capture. If we would rather not carry the heuristic at all, deprecating ticker_hints is a reasonable alternative; happy to discuss which way you'd prefer.
Note: I pushed a small chore: normalize CRLF line endings to LF commit to this branch. The files were saved with CRLF, which made the diff look like a full-file rewrite; it now reads as ~+89/-4 against the merge base.
Summary
EXCHANGE: SYMBOLraw values for additional list membersRelated Issue
Closes #108
Verification
pytest -o addopts='' tests/preprocess/test_news.py -q- 20 passed, 8 subtests passedruff format --check .- passedruff check .- passedbasedpyright- passed with 0 errorslint-imports- 7 contracts kept, 0 brokenpython examples/preprocess/02_news_ticker_lists.py- emitted bothEVEXandEVEXWpytest --covon Windows reached 84.05% coverage with 322 passed and 13 pre-existing platform failures involving SQLite temporary-file locks and LiteParse artifact paths; this draft relies on the repository's Linux CI for the required deterministic verification.Checklist
type(scope): summary.bash scripts/verify.shpasses locally; Windows-specific existing failures are disclosed above and Linux CI is pending.